From: Camila Ayres Date: Wed, 4 Dec 2024 20:15:10 +0000 (+0100) Subject: Improve text returned when checking VFS availability. X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~2^2~73^2 X-Git-Url: https://dgit.raspbian.org/%22http://www.example.com/cgi/success/%22http:/www.example.com/cgi/success?a=commitdiff_plain;h=f62efca6c477857d6d056cce35b43d561b962047;p=nextcloud-desktop.git Improve text returned when checking VFS availability. - Display more clear error when adding local sync folder and when selecting remote folder. - Modernize FolderWizardRemotePath::isComplete(). - Display the local folder path when warning that folder is already being synced. Signed-off-by: Camila Ayres --- diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp index 4f22bfedb..1d0b6d23d 100644 --- a/src/common/vfs.cpp +++ b/src/common/vfs.cpp @@ -71,16 +71,16 @@ Result Vfs::checkAvailability(const QString &path, Vfs::Mode mode #ifdef Q_OS_WIN if (mode == Mode::WindowsCfApi) { const auto info = QFileInfo(path); - if (QDir(info.canonicalFilePath()).isRoot()) { - return tr("The Virtual filesystem feature does not support a drive as sync root"); + if (QDir(info.canonicalPath()).isRoot()) { + return tr("Please choose a different location. %1 is a drive. It doesn't support virtual files.").arg(path); } - const auto fs = FileSystem::fileSystemForPath(info.absoluteFilePath()); - if (fs != QLatin1String("NTFS")) { - return tr("The Virtual filesystem feature requires a NTFS file system, %1 is using %2").arg(path, fs); + if (const auto fileSystemForPath = FileSystem::fileSystemForPath(info.absoluteFilePath()); + fileSystemForPath != QLatin1String("NTFS")) { + return tr("Please choose a different location. %1 isn't a NTFS file system. It doesn't support virtual files.").arg(path); } const auto type = GetDriveTypeW(reinterpret_cast(QDir::toNativeSeparators(info.absoluteFilePath().mid(0, 3)).utf16())); if (type == DRIVE_REMOTE) { - return tr("The Virtual filesystem feature is not supported on network drives"); + return tr("Please choose a different location. %1 is a network drive. It doesn't support virtual files.").arg(path); } } #else diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 126a39d5c..fb9427c7d 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -197,11 +197,11 @@ bool Folder::checkLocalPath() QString error; // Check directory again if (!FileSystem::fileExists(_definition.localPath, fi)) { - error = tr("Local folder %1 does not exist.").arg(_definition.localPath); + error = tr("Please choose a different location. The folder %1 doesn't exist.").arg(_definition.localPath); } else if (!fi.isDir()) { - error = tr("%1 should be a folder but is not.").arg(_definition.localPath); + error = tr("Please choose a different location. %1 isn't a valid folder.").arg(_definition.localPath); } else if (!fi.isReadable()) { - error = tr("%1 is not readable.").arg(_definition.localPath); + error = tr("Please choose a different location. %1 isn't a readable folder.").arg(_definition.localPath); } if (!error.isEmpty()) { _syncResult.appendErrorString(error); diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index ff84383c5..69c7dfeab 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1799,7 +1799,7 @@ QString FolderMan::trayTooltipStatusString(SyncResult::Status syncStatus, bool h static QString checkPathValidityRecursive(const QString &path) { if (path.isEmpty()) { - return FolderMan::tr("No valid folder selected!"); + return FolderMan::tr("Please choose a different location. The selected folder isn't valid."); } #ifdef Q_OS_WIN @@ -1807,18 +1807,20 @@ static QString checkPathValidityRecursive(const QString &path) #endif const QFileInfo selFile(path); if (numberOfSyncJournals(selFile.filePath()) != 0) { - return FolderMan::tr("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(selFile.filePath())); + return FolderMan::tr("Please choose a different location. %1 is already being used as a sync folder.").arg(QDir::toNativeSeparators(selFile.filePath())); } if (!FileSystem::fileExists(path)) { QString parentPath = selFile.dir().path(); - if (parentPath != path) + if (parentPath != path) { return checkPathValidityRecursive(parentPath); - return FolderMan::tr("The selected path does not exist!"); + } + + return FolderMan::tr("Please choose a different location. The path %1 doesn't exist.").arg(QDir::toNativeSeparators(selFile.filePath())); } if (!FileSystem::isDir(path)) { - return FolderMan::tr("The selected path is not a folder!"); + return FolderMan::tr("Please choose a different location. The path %1 isn't a folder.").arg(QDir::toNativeSeparators(selFile.filePath())); } #ifdef Q_OS_WIN @@ -1826,12 +1828,12 @@ static QString checkPathValidityRecursive(const QString &path) // isWritable() doesn't cover all NTFS permissions // try creating and removing a test file, and make sure it is excluded from sync if (!Utility::canCreateFileInPath(path)) { - return FolderMan::tr("You have no permission to write to the selected folder!"); + return FolderMan::tr("Please choose a different location. You don't have enough permissions to write to %1.", "folder location").arg(QDir::toNativeSeparators(selFile.filePath())); } } #else if (!FileSystem::isWritable(path)) { - return FolderMan::tr("You have no permission to write to the selected folder!"); + return FolderMan::tr("Please choose a different location. You don't have enough permissions to write to %1.", "folder location").arg(QDir::toNativeSeparators(selFile.filePath())); } #endif return {}; @@ -1884,17 +1886,15 @@ QPair FolderMan::checkPathValidityForNew bool differentPaths = QString::compare(folderDir, userDir, cs) != 0; if (differentPaths && folderDir.startsWith(userDir, cs)) { result.first = FolderMan::PathValidityResult::ErrorContainsFolder; - result.second = tr("The local folder %1 already contains a folder used in a folder sync connection. " - "Please pick another one!") + result.second = tr("Please choose a different location. %1 is already being used as a sync folder.") .arg(QDir::toNativeSeparators(path)); return result; } if (differentPaths && userDir.startsWith(folderDir, cs)) { result.first = FolderMan::PathValidityResult::ErrorContainedInFolder; - result.second = tr("The local folder %1 is already contained in a folder used in a folder sync connection. " - "Please pick another one!") - .arg(QDir::toNativeSeparators(path)); + result.second = tr("Please choose a different location. %1 is already contained in a folder used as a sync folder.") + .arg(QDir::toNativeSeparators(path)); return result; } @@ -1908,8 +1908,9 @@ QPair FolderMan::checkPathValidityForNew if (serverUrl == folderUrl) { result.first = FolderMan::PathValidityResult::ErrorNonEmptyFolder; - result.second = tr("There is already a sync from the server to this local folder. " - "Please pick another local folder!"); + result.second = tr("Please choose a different location. %1 is already being used as a sync folder for %2.", "folder location, server url") + .arg(QDir::toNativeSeparators(path), + serverUrl.toString()); return result; } } diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp index 6a45da8a1..3561699d6 100644 --- a/src/gui/folderwizard.cpp +++ b/src/gui/folderwizard.cpp @@ -61,18 +61,18 @@ namespace OCC { QString FormatWarningsWizardPage::formatWarnings(const QStringList &warnings) const { - QString ret; + QString formattedWarning; if (warnings.count() == 1) { - ret = tr("Warning: %1").arg(warnings.first()); + formattedWarning = warnings.first(); } else if (warnings.count() > 1) { - ret = tr("Warning:") + "
    "; - Q_FOREACH (QString warning, warnings) { - ret += QString::fromLatin1("
  • %1
    • ").arg(warning); + formattedWarning = "
      "; + for (const auto &warning : warnings) { + formattedWarning += QString::fromLatin1("
    • %1
      • ").arg(warning); } - ret += "
    "; + formattedWarning += "
"; } - return ret; + return formattedWarning; } FolderWizardLocalPath::FolderWizardLocalPath(const AccountPtr &account) @@ -484,34 +484,39 @@ FolderWizardRemotePath::~FolderWizardRemotePath() = default; bool FolderWizardRemotePath::isComplete() const { - if (!_ui.folderTreeWidget->currentItem()) + if (!_ui.folderTreeWidget->currentItem()) { return false; + } - QStringList warnStrings; - QString dir = _ui.folderTreeWidget->currentItem()->data(0, Qt::UserRole).toString(); - if (!dir.startsWith(QLatin1Char('/'))) { - dir.prepend(QLatin1Char('/')); + auto targetPath = _ui.folderTreeWidget->currentItem()->data(0, Qt::UserRole).toString(); + if (!targetPath.startsWith(QLatin1Char('/'))) { + targetPath.prepend(QLatin1Char('/')); } - wizard()->setProperty("targetPath", dir); + wizard()->setProperty("targetPath", targetPath); - Folder::Map map = FolderMan::instance()->map(); - Folder::Map::const_iterator i = map.constBegin(); - for (i = map.constBegin(); i != map.constEnd(); i++) { - auto *f = static_cast(i.value()); - if (f->accountState()->account() != _account) { + for (const auto folder : std::as_const(FolderMan::instance()->map())) { + if (folder->accountState()->account() != _account) { continue; } - QString curDir = f->remotePathTrailingSlash(); - if (QDir::cleanPath(dir) == QDir::cleanPath(curDir)) { - warnStrings.append(tr("This folder is already being synced.")); - } else if (dir.startsWith(curDir)) { - warnStrings.append(tr("You are already syncing %1, which is a parent folder of %2.").arg(Utility::escape(curDir), Utility::escape(dir))); - } else if (curDir.startsWith(dir)) { - warnStrings.append(tr("You are already syncing %1, which is a subfolder of %2.").arg(Utility::escape(curDir), Utility::escape(dir))); + + const auto remoteDir = folder->remotePathTrailingSlash(); + const auto localDir = folder->cleanPath(); + if (QDir::cleanPath(targetPath) == QDir::cleanPath(remoteDir)) { + showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(remoteDir), Utility::escape(localDir))); + break; + } + + if (targetPath.startsWith(remoteDir)) { + showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(targetPath), Utility::escape(localDir))); + break; + } + + if (remoteDir.startsWith(targetPath)) { + showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(remoteDir), Utility::escape(localDir))); + break; } } - showWarn(formatWarnings(warnStrings)); return true; } @@ -625,7 +630,10 @@ bool FolderWizardSelectiveSync::validatePage() if (useVirtualFiles) { const auto availability = Vfs::checkAvailability(wizard()->field(QStringLiteral("sourceFolder")).toString(), mode); if (!availability) { - auto msg = new QMessageBox(QMessageBox::Warning, tr("Virtual files are not available for the selected folder"), availability.error(), QMessageBox::Ok, this); + auto msg = new QMessageBox(QMessageBox::Warning, + tr("Virtual files are not supported at the selected location"), + availability.error(), + QMessageBox::Ok, this); msg->setAttribute(Qt::WA_DeleteOnClose); msg->open(); return false; diff --git a/src/gui/wizard/owncloudadvancedsetuppage.cpp b/src/gui/wizard/owncloudadvancedsetuppage.cpp index f515e1c71..bb51e4474 100644 --- a/src/gui/wizard/owncloudadvancedsetuppage.cpp +++ b/src/gui/wizard/owncloudadvancedsetuppage.cpp @@ -395,7 +395,10 @@ bool OwncloudAdvancedSetupPage::validatePage() if (useVirtualFileSync()) { const auto availability = Vfs::checkAvailability(localFolder(), bestAvailableVfsMode()); if (!availability) { - auto msg = new QMessageBox(QMessageBox::Warning, tr("Virtual files are not available for the selected folder"), availability.error(), QMessageBox::Ok, this); + auto msg = new QMessageBox(QMessageBox::Warning, + tr("Virtual files are not supported at the selected location"), + availability.error(), + QMessageBox::Ok, this); msg->setAttribute(Qt::WA_DeleteOnClose); msg->open(); return false; diff --git a/test/testfolderman.cpp b/test/testfolderman.cpp index f40c3c7e4..c83737828 100644 --- a/test/testfolderman.cpp +++ b/test/testfolderman.cpp @@ -313,8 +313,8 @@ private slots: // The following both fail because they are already sync folders even if for another account QUrl url3("http://anotherexample.org"); url3.setUserName("dummy"); - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(dirPath + "/sub/ownCloud1"))); - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(dirPath + "/ownCloud2"))); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3).second, QString("Please choose a different location. %1 is already being used as a sync folder.").arg(QDir::toNativeSeparators(dirPath + "/sub/ownCloud1"))); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2", url3).second, QString("Please choose a different location. %1 is already being used as a sync folder.").arg(QDir::toNativeSeparators(dirPath + "/ownCloud2"))); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath).second.isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder").second.isNull()); @@ -337,7 +337,7 @@ private slots: // link 3 points to an existing sync folder. To make it fail, the account must be the same QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3", url2).second.isNull()); // while with a different account, this is fine - QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3).second, QString("The folder %1 is used in a folder sync connection!").arg(dirPath + "/link3")); + QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3).second, QString("Please choose a different location. %1 is already being used as a sync folder.").arg(dirPath + "/link3")); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link4").second.isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder").second.isNull());